Provide method to access GT and CT HW jet objects from l1t::PFJet by thesps · Pull Request #1088 · cms-l1t-offline/cmssw

thesps · 2023-03-31T11:50:07Z

As requested by GT emulator developers, this PR provides accessors to retrieve the hardware encoded jet object from the l1t::PFJet. Since emulator consumers of l1t::PFJet may be in either Correlator or Global Trigger systems, which have different coordinate scales, accessors to both types of objects are provided. Consumers have been updated to select the CT or GT object where appropriate.

The L1MHtProducer MHT emulator is also updated to set the integer pt and phi fields of the l1t::EtSum product such that GT can retrieve those fields for emulation.

@EmyrClement @gpetruc

gpetruc

I would suggest some modifications for code cleanliness

DataFormats/L1TParticleFlow/interface/PFJet.h

L1Trigger/Phase2L1ParticleFlow/plugins/L1MHtPFProducer.cc

DataFormats/L1TParticleFlow/interface/PFJet.h

triggerDoctor · 2023-03-31T14:46:21Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info	Value
return code	`0`
command	`cmsenv && scram b -j 8`

triggerDoctor · 2023-03-31T14:50:31Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 code-checks && scram b -k -j 8 code-checks`

triggerDoctor · 2023-03-31T14:50:48Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found 1 files that did not meet formatting requirements:

DataFormats/L1TParticleFlow/interface/PFJet.h

Please run scram b code-format to auto-apply code formatting

aloeliger · 2023-03-31T14:52:36Z

@thesps Could you please fix the code formatting?

aloeliger · 2023-03-31T14:55:39Z

I think @gpetruc 's comments are probably correct (and better than what I could have added, thanks!)

triggerDoctor · 2023-03-31T15:09:15Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

This PR passes available unit tests!

Info	Value
return code	`0`
command	`cmsenv && scram b runtests`

thesps · 2023-03-31T15:19:39Z

Thanks for the feedback both, I've implemented the suggestions.

triggerDoctor · 2023-03-31T15:27:10Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info	Value
return code	`0`
command	`cmsenv && scram b -j 8`

triggerDoctor · 2023-03-31T15:31:17Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 code-checks && scram b -k -j 8 code-checks`

I found no issues with the headers!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 check-headers`

triggerDoctor · 2023-03-31T15:31:34Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no files with code format issues!

triggerDoctor · 2023-03-31T15:35:22Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

This PR passes available unit tests!

Info	Value
return code	`0`
command	`cmsenv && scram b runtests`

triggerDoctor · 2023-03-31T17:54:05Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

This PR does not change the EDM content of testing ntuples.

gpetruc · 2023-03-31T18:04:50Z

Right, I had missed that it was an ap_fixed. Yes, bits_to_uint64 is better. Il Ven 31 Mar 2023, 17:19 Sioni Summers ***@***.***> ha scritto:

…

***@***.**** commented on this pull request. ------------------------------ In L1Trigger/Phase2L1ParticleFlow/plugins/L1MHtPFProducer.cc <#1088 (comment)> : > @@ -84,8 +84,8 @@ std::vector<l1t::EtSum> L1MhtPfProducer::convertHWToEDM(l1ct::Sum hwSums) const mhtVector.SetPhi(l1ct::Scales::floatPhi(hwSums.hwPhi)); mhtVector.SetEta(0); - l1t::EtSum ht(htVector, l1t::EtSum::EtSumType::kTotalHt); - l1t::EtSum mht(mhtVector, l1t::EtSum::EtSumType::kMissingHt); + l1t::EtSum ht(htVector, l1t::EtSum::EtSumType::kTotalHt, hwSums.hwSumPt.V); + l1t::EtSum mht(mhtVector, l1t::EtSum::EtSumType::kMissingHt, hwSums.hwPt.V, 0, hwSums.hwPhi.V); This is the only comment I implemented differently. .to_int() and .V do different things. e.g. for some ap_fixed<4,2> x = 1.5;, .to_int() basically casts to int yielding 1, while .V gives the underlying bits of 6 in this case. In any case, I found a method bits_to_uint64() that gives access to the underlying bits, and is more readable. For these pt_t variables it would be like doing hwPt * 4. — Reply to this email directly, view it on GitHub <#1088 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABDNWR5ZHKIY7ZZ5F43YUKDW63YXLANCNFSM6AAAAAAWORPWGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

aloeliger · 2023-03-31T18:39:10Z

@epalencia I think I am okay with this. Anything from your side? If not @thesps would you please open a PR for this to central CMSSW? If possible we would like to keep these PRs synchronized.

epalencia · 2023-03-31T19:58:37Z

Nothing from my side. @thesps, please open the corresponding PR in master.

thesps · 2023-04-03T09:30:33Z

I'll open it. Are we keeping this open as well?

aloeliger · 2023-04-03T09:32:14Z

We'll merge this when the version to master is pulled in (to not desynchronize branches). If any additional commits are made there, we would appreciate cherry-picking them back here so we can provide a consistent recipe in 12_5_2 (for use on MC) as will be available eventually in central CMSSW.

aloeliger · 2023-04-04T20:17:09Z

Just a memo to myself: This is delayed until the mega-correlator-PR started by Cecile and February and picked up by me gets merged, at which point this can go into central.

…date consumers to pick the appropriate HW object where needed Address review comments Fix HT emulator input collection and output encoding

thesps · 2023-04-13T13:04:07Z

Updates:

HT emulation was consuming the wrong collection, that's updated
HT emulation was producing wrongly encoded outputs, they are fixed

HaarigerHarald · 2023-04-13T13:20:10Z

@thesps will this approach (using hwPT, hwPhi) also be implemented for the Correlator MET?

triggerDoctor · 2023-04-13T13:32:50Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info	Value
return code	`0`
command	`cmsenv && scram b -j 8`

triggerDoctor · 2023-04-13T13:37:35Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 code-checks && scram b -k -j 8 code-checks`

I found no issues with the headers!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 check-headers`

triggerDoctor · 2023-04-13T13:37:58Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no files with code format issues!

Info	Value
return code	`0`
command	`cmsenv && scram b -k -j 8 code-format-all`

aloeliger · 2023-05-04T09:17:07Z

@thesps The correlator PR that this is dependent upon should be merged now. Please open a PR containing this commit to central CMSSW. If any commits are requested in review there, we would appreciate them being cherry-picked back here. Because the correlator PR had to be changed w.r.t. this branch, merge issues may still exist, please let us know if there are any issues.

epalencia · 2023-05-12T13:40:55Z

Corresponding PR in master, cms-sw#41590, merged already.

epalencia · 2023-05-12T13:50:27Z

Tagged as l1t-phase2-v61.

gpetruc reviewed Mar 31, 2023

View reviewed changes

aloeliger added Phase-2 Pertains to phase-2 development Data types Anything related to development of datatypes for L1 use labels Mar 31, 2023

thesps force-pushed the p2-1252p1-scj2gt branch from 1e7e912 to 25885d9 Compare March 31, 2023 15:13

aloeliger added the GT Necessary label Apr 5, 2023

Provide method to access GT and CT HW jet objects from l1t::PFJet, up…

d6e23fd

…date consumers to pick the appropriate HW object where needed Address review comments Fix HT emulator input collection and output encoding

thesps force-pushed the p2-1252p1-scj2gt branch from 25885d9 to d6e23fd Compare April 13, 2023 12:54

epalencia merged commit 62a07dd into cms-l1t-offline:phase2-l1t-integration-1252patch1 May 12, 2023

gpetruc mentioned this pull request May 24, 2023

Sync to cms-l1t-offline tag v64 + egamma sorter p2l1pfp/cmssw#117

Merged

Comments

Conversation

thesps commented Mar 31, 2023

Uh oh!

gpetruc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

aloeliger commented Mar 31, 2023

Uh oh!

aloeliger commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

thesps commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

triggerDoctor commented Mar 31, 2023

Uh oh!

gpetruc commented Mar 31, 2023 via email

Uh oh!

aloeliger commented Mar 31, 2023

Uh oh!

epalencia commented Mar 31, 2023

Uh oh!

thesps commented Apr 3, 2023

Uh oh!

aloeliger commented Apr 3, 2023

Uh oh!

aloeliger commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thesps commented Apr 13, 2023

Uh oh!

HaarigerHarald commented Apr 13, 2023

Uh oh!

triggerDoctor commented Apr 13, 2023

Uh oh!

triggerDoctor commented Apr 13, 2023

Uh oh!

triggerDoctor commented Apr 13, 2023

Uh oh!

aloeliger commented May 4, 2023

Uh oh!

epalencia commented May 12, 2023

Uh oh!

epalencia commented May 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thesps commented Mar 31, 2023 •

edited

Loading

aloeliger commented Apr 4, 2023 •

edited

Loading